-
Notifications
You must be signed in to change notification settings - Fork 490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reworking Policy vs. Filter Documentation #880
Reworking Policy vs. Filter Documentation #880
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Just a few tiny nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly disagree with this, it leads to an extremely inconsistent experience for users.
class fields, which offer a more streamlined UX than custom policy | ||
attachment. | ||
|
||
#### 2. Custom filters and policies should not overlap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems in violation of our goals of policy attachment. This means any given configuration can apply to {gateway, route, service} or {route rule, route backend}, but nothing can apply to both. And if its in one category, it has to have one shape and is a backref, and in the other category its in another shape and is a forward ref.
This inconsistency is extremely confusing to users and implementors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems in violation of our goals of policy attachment
This inconsistency is extremely confusing to users and implementors.
I personally disagree. I think allowing/encouraging the same resources/config to be attached with both filters and policy would get more confusing.
The way I look at it, it's potentially confusing to have both policy and custom filters, but this helps limit that by differentiating the two a bit more clearly. I think there are legitimate use cases where custom filters will be easier to work with.
This means any given configuration can apply to {gateway, route, service} or {route rule, route backend}, but nothing can apply to both
That's roughly true, but with policy you still have a lot of flexibility. It's entirely possible to split out Route rules into different Routes if needed for policy attachment. Although not ideal, it does make policy attachment fairly capable. When combined with the ability for policy to attach directly to backends, it seems like there aren't many remaining gaps in policy capabilities.
412098a
to
6d3325c
Compare
6d3325c
to
bf1908a
Compare
I'm OK in general with giving guidance on how we think implementations should approach this, but let's be prepared for the guidance to be wrong :) It's possible for implementations to come up with features where it makes sense to break or modify this guidance. That's OK, since it's more important for the feature to be internally consistent and useful. We can revisit and update guidance as teams gain more implementation experience. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: howardjohn, jpeach, robscott The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I think that the guidance we have is good for now. We can always update if we need to. /lgtm /hold cancel |
What type of PR is this?
/kind documentation
What this PR does / why we need it:
This adds some clarification in our docs based on an earlier Slack thread around if there was still value in custom filters now that we have policy attachment.
Does this PR introduce a user-facing change?: